Skip to content

GH-48799: [C++] Improve SharedExclusiveChecker error messages #48800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
pitrou merged 1 commit into from
Jan 12, 2026
Merged

GH-48799: [C++] Improve SharedExclusiveChecker error messages #48800

pitrou merged 1 commit into from
Jan 12, 2026
+6 −5

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Jan 9, 2026 •
edited by github-actions bot
Loading

Rationale for this change

The XXX comment in SharedExclusiveChecker noted that error messages were too generic and didn't describe the actual operations involved.

Shared lock:

ReadAt():

arrow/cpp/src/arrow/io/concurrency.h

Lines 210 to 212 in 7be5a89

Result<int64_t> ReadAt(int64_t position, int64_t nbytes, void* out) final {
auto guard = lock_.shared_guard();
return derived()->DoReadAt(position, nbytes, out);

arrow/cpp/src/arrow/io/concurrency.h

Lines 215 to 217 in 7be5a89

Result<std::shared_ptr<Buffer>> ReadAt(int64_t position, int64_t nbytes) final {
auto guard = lock_.shared_guard();
return derived()->DoReadAt(position, nbytes);

GetSize():

arrow/cpp/src/arrow/io/concurrency.h

Lines 199 to 201 in 7be5a89

Result<int64_t> GetSize() final {
auto guard = lock_.shared_guard();
return derived()->DoGetSize();

Exclusive lock:

Read():

arrow/cpp/src/arrow/io/concurrency.h

Lines 109 to 111 in 7be5a89

Result<int64_t> Read(int64_t nbytes, void* out) final {
auto guard = lock_.exclusive_guard();
return derived()->DoRead(nbytes, out);

arrow/cpp/src/arrow/io/concurrency.h

Lines 114 to 116 in 7be5a89

Result<std::shared_ptr<Buffer>> Read(int64_t nbytes) final {
auto guard = lock_.exclusive_guard();
return derived()->DoRead(nbytes);

arrow/cpp/src/arrow/io/concurrency.h

Lines 179 to 181 in 7be5a89

Result<int64_t> Read(int64_t nbytes, void* out) final {
auto guard = lock_.exclusive_guard();
return derived()->DoRead(nbytes, out);

arrow/cpp/src/arrow/io/concurrency.h

Lines 184 to 186 in 7be5a89

Result<std::shared_ptr<Buffer>> Read(int64_t nbytes) final {
auto guard = lock_.exclusive_guard();
return derived()->DoRead(nbytes);

Seek():

arrow/cpp/src/arrow/io/concurrency.h

Lines 194 to 196 in 7be5a89

Status Seek(int64_t position) final {
auto guard = lock_.exclusive_guard();
return derived()->DoSeek(position);

Tell():

arrow/cpp/src/arrow/io/concurrency.h

Lines 104 to 106 in 7be5a89

Result<int64_t> Tell() const final {
auto guard = lock_.exclusive_guard();
return derived()->DoTell();

arrow/cpp/src/arrow/io/concurrency.h

Lines 174 to 176 in 7be5a89

Result<int64_t> Tell() const final {
auto guard = lock_.exclusive_guard();
return derived()->DoTell();

Peek():

arrow/cpp/src/arrow/io/concurrency.h

Lines 119 to 121 in 7be5a89

Result<std::string_view> Peek(int64_t nbytes) final {
auto guard = lock_.exclusive_guard();
return derived()->DoPeek(nbytes);

arrow/cpp/src/arrow/io/concurrency.h

Lines 189 to 191 in 7be5a89

Result<std::string_view> Peek(int64_t nbytes) final {
auto guard = lock_.exclusive_guard();
return derived()->DoPeek(nbytes);

Close():

arrow/cpp/src/arrow/io/concurrency.h

Lines 94 to 96 in 7be5a89

Status Close() final {
auto guard = lock_.exclusive_guard();
return derived()->DoClose();

arrow/cpp/src/arrow/io/concurrency.h

Lines 164 to 166 in 7be5a89

Status Close() final {
auto guard = lock_.exclusive_guard();
return derived()->DoClose();

Abort():

arrow/cpp/src/arrow/io/concurrency.h

Lines 99 to 101 in 7be5a89

Status Abort() final {
auto guard = lock_.exclusive_guard();
return derived()->DoAbort();

arrow/cpp/src/arrow/io/concurrency.h

Lines 169 to 171 in 7be5a89

Status Abort() final {
auto guard = lock_.exclusive_guard();
return derived()->DoAbort();

What changes are included in this PR?

Improved three error messages in SharedExclusiveChecker to explicitly list the I/O operations involved:

  • Shared lock operations: ReadAt, GetSize
  • Exclusive lock operations: Read, Seek, Tell, Peek, Close, Abort

Are these changes tested?

I manually tested all of combinations.

Are there any user-facing changes?

No.

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

⚠️ GitHub issue #48799 has been automatically assigned in GitHub to PR creator.

@HyukjinKwon HyukjinKwon force-pushed the improve-io-error-messages branch from 7be5a89 to 71c032a Compare January 9, 2026 07:33
pitrou
pitrou approved these changes Jan 12, 2026
View reviewed changes
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, looks like a simple and obvious improvement

@pitrou pitrou merged commit 6657a92 into apache:main Jan 12, 2026
49 checks passed
@pitrou pitrou removed the awaiting review Awaiting review label Jan 12, 2026
@pitrou pitrou mentioned this pull request Jan 12, 2026
Closed
@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Jan 12, 2026
@conbench-apache-arrow
Copy link

conbench-apache-arrow bot commented Jan 13, 2026

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 6657a92.

There weren't enough matching historic benchmark results to make a call on whether there were regressions.

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou pitrou approved these changes

Assignees

No one assigned

Labels

awaiting committer review Awaiting committer review Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@HyukjinKwon @pitrou